fix(plugin-security): the sys_permission_set duplicate-name refusal carries UNIQUE_VIOLATION, and the packaged-set lock answers first - #19437
Conversation
…TION, and the packaged-set lock speaks first
The data door's duplicate-name refusal on `sys_permission_set` threw a bare
`Error` with `.status = 409` and no `.code`, so the flat `{ error, code }`
responder — which invents nothing for a producer that declared nothing — put
prose on the wire, against ADR-0112's 2026-08-17 amendment.
It also ran BEFORE `assertPermissionSetNotPackageDeclared`. A package-declared
set has a projected row, so its name is duplicate and locked at once, and the
admin who opens the Clone dialog on a packaged set and types the base set's own
name never reached NOT_OVERRIDABLE — the refusal that names the remedy.
`UNIQUE_VIOLATION` is reused rather than minted: `sys_permission_set` declares
`{ fields: ['name'], unique: 'organization' }`, so the same collision already
answers 409 UNIQUE_VIOLATION when the index catches it instead of this
pre-check. The code is stamped through an exported `*_CODE` constant so
`check:error-code-provenance` can see this package as an emitter; the ledger
gains the matching provenance row.
Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
…the lock's precedence Five cases on `createPermissionSetWriteThrough`'s insert leg: the ordinary duplicate's ADR-0112 envelope (`code` + `status`, never a bare throw), the packaged-set case answering the lock rather than the conflict, the negative control that ordinary duplicates did NOT become NOT_OVERRIDABLE, the happy path, and the fail-closed `unknown` corner the reorder moved. The engine double holds the caller's `limit` and opens `update` with `assertEngineUpdateDispatch`, and the new pinned seams are registered in `scripts/engine-double-contract.pinned.json`. Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 139 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 060b7340722e298e00d0e0fea330f3dd2167a2c1 && git checkout 060b7340722e298e00d0e0fea330f3dd2167a2c1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 57ceb9d6d2af35ebf83d0f8cc396cb612540af74 f51a59889faf0a8e494b842385fcc8feeb1eb1e7 && git checkout -B drift-repro 57ceb9d6d2af35ebf83d0f8cc396cb612540af74 && git merge --no-ff f51a59889faf0a8e494b842385fcc8feeb1eb1e7
node scripts/docs-audit/affected-docs.mjs --json 57ceb9d6d2af35ebf83d0f8cc396cb612540af74
|
…es, and state the reorder's cost The clause-② declaration is disjunctive and only the accept-set limb had been measured. `ERROR_CODE_LEDGER` is exported `as const satisfies`, so its published type carries literal tuples and the provenance row moves `typeof ERROR_CODE_LEDGER['@objectstack/plugin-security']` from a 3-tuple to a 4-tuple — the public-surface limb. A purely additive widening of a published surface takes at least `minor`; `@objectstack/plugin-security` stays `patch`. Three wording repairs in the same file: the fail-closed corner names its code (`403 NOT_OVERRIDABLE`), the "what did NOT move" claim is qualified to the case where provenance resolves, and the reorder's undeclared side effect is stated — `getMetaItemLayered` is now evaluated unconditionally, so a duplicate insert pays one extra metadata round trip and the duplicate path is coupled to metadata-layer reachability. Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF Co-authored-by: Claude <noreply@anthropic.com>
契约复核 provenance —— 达档复核 PASS 在案,
|
| 步 | 做了什么 | 现验 |
|---|---|---|
| 1 | 卡侧申报 no → yes |
Clause-②-correction: 5752398762(评论 5753070284),门读作 yes |
| 2 | PR 正文申报行 → yes |
PATCH 后回读为真 |
| 3 | changeset @objectstack/spec patch → minor |
现读 frontmatter 已 minor,plugin-security 留 patch |
| 4 | needs:contract-review 双载体 |
已挂,并由本评论同笔剥 |
⭐ 决定性的一条:git diff --name-only 72d68b905 f51a59889 -- 'packages/**' 'scripts/**' 为空 ⇒ 复核审过的代码逐字节未动,这一轮只改了 changeset 文本与 PR 正文。所以复核对代码的四项结论不需要重取。
条款②的最终值与理由
Clause-②: yes —— 理由 ⛔ 不是接受集增长。去重并集实测 282 → 282,added/removed 皆空(本席与复核各用一种方法独立测得,本席按数组元素行计并排除注释行,复核交 JS 解析器求值)。yes 的依据是析取式的另一条肢:ERROR_CODE_LEDGER 是已发布导出(api-surface/api.json:325)且 as const satisfies(error-code-ledger.zod.ts:1286),新增一元素使该 key 的字面量 tuple 3 元组 → 4 元组,已发布类型变了 ⇒ 扩大公开面。在档同形先例 #17909 / PR #17924 以同一理由落地。
⇒ changeset 因此必须 ≥ minor,这是申报的硬耦合后果,⛔ 不是可选项。
入队前七闸,全部有仪器背书
| 闸 | 读数 |
|---|---|
CI(按 check-name 取最新,钉 f51a59889) |
32 success · 3 skipped · 0 失败 · 0 挂起 |
三条 cancelled 的 check |
各有更晚的同名 run 为 success ⇒ 被取代,非未决 |
check-expected-skips |
exit 0 —— 5 个 skip 全在名册 |
check-clause2-carriers --pair 19437 |
exit 0(C2-CORRECTION 读作 yes) |
check-governed-merges |
NOT governed,502 行 < 5000 |
.gitattributes os-regen |
当场从 origin/main 重读(18 条路径),本 PR 六个文件无一命中 |
| 达档复核 PASS | 本评论即记录 |
node_modules 未安装,其消融结论由读断言 + 读控制流推出,⛔ 非实跑;dev 侧的实跑读数与之同向。② 本席本地工作树停在 1739f71879,故上述 scripts/pm/* 门禁均改用 origin/main 抽出的当前版本重跑 —— 本班已因陈旧工作树出过一次行号错(5751763031)。
Generated by Claude Code
Fixes #19307
Clause-②: yes
The data door's duplicate-name refusal on
sys_permission_sethad two halves, both reproduced on today's head (c33707933) before any edit and re-measured after, onexamples/app-showcasewith a seeded admin over a cookie session.The two halves
1. No machine-readable
code. The insert leg threw a bareErrorcarrying.status = 409and no.code. The flat{ error, code }responder invents nothing for a producer that declared nothing, so the caller got prose — against ADR-0112's 2026-08-17 amendment (#9232), under which the flat door carries the closed member too.2. ⭐ It ran BEFORE
assertPermissionSetNotPackageDeclared. A package-declared set has a projected row, so its name is duplicate AND locked at once. The admin who opens the Clone dialog on a packaged set and types the base set's own name — the single most likely thing to type — gotalready exists, which names no remedy, and never reachedNOT_OVERRIDABLE, which names the clone path.Live readings — five legs, same script, before and after
Script:
POST /api/v1/data/sys_permission_setas the seeded admin; full text in the report comment on #19307.c33707933)showcase_manager409·{"error":"[Security] permission set 'showcase_manager' already exists","object":"sys_permission_set"}— nocodekey403·"code":"NOT_OVERRIDABLE", message: "…Choose a different name for your set, or clone 'showcase_manager' (the "Clone" action…)"201201(unchanged)409· nocode409·"code":"UNIQUE_VIOLATION", message byte-identicalPATCH, same resource401·"code":"UNAUTHENTICATED"401·"code":"UNAUTHENTICATED"(unchanged)PATCHthe packaged set (non-duplicate route)403·"code":"NOT_OVERRIDABLE"Legs 4 and 5 are what make legs 1 and 3 readings rather than constants: the flat door already varied its
codeby path, so the absence was this producer's and never the door's.Why
UNIQUE_VIOLATION, reused and not mintedsys_permission_setdeclares{ fields: ['name'], unique: 'organization' }, and the reading recorded on that index itself (#8554) isorg_yi 409 UNIQUE_VIOLATION. So the platform already answers this exact collision with this exact envelope whenever the index catches it instead of the projection's pre-check. A second spelling here would make one condition answer two envelopes depending only on which layer got there first — the drift@objectstack/restand@objectstack/driver-memorydeliberately registered the SAME code to avoid.RESOURCE_CONFLICT(the standard member 409 derives from) is what the door supplies for a producer that named no condition; using it would be that second spelling.The
packages/spectouch is one provenance rowcheck:error-code-provenancerecognisesobjlit,assignand*_CODEconstdefstamp sites and states in its own bounds line that it is blind to class fields. Written as a class-field literal, this package would have become an unlisted EMITTER of a registered code with every gate in the repo green — the invisibility the ledger header names ("no admission rule checks WHO emits"), found by hand three times already (#7504 / #13254 / #13353). So the code is stamped through an exportedPERMISSION_SET_NAME_CONFLICT_CODE, which puts the emitter inside the gate's field of view, and the ledger gains the matching row under@objectstack/plugin-security.Measured red then green, in that order:
*_CODEconstant, no ledger row: gate exit 1 — "@objectstack/plugin-security stamps 'UNIQUE_VIOLATION' (constdef) at packages/plugins/plugin-security/src/errors.ts:372 — not listed under its own owner key";*_CODEconstant + the row: gate exit 0, 339 stamp sites, 322 listed.It is provenance, not identity, and that is measured rather than asserted. The deduped ledger union is 282 distinct codes before and 282 after, added [] / removed [] —
UNIQUE_VIOLATIONwas already a member under two other packages.pnpm --filter @objectstack/spec check:generatedreports all 15 generated artifacts up to date, which supports ONE claim only — no regeneration is MISSING. ⛔ It does NOT show the published TYPE is unchanged, and here it is not:ERROR_CODE_LEDGERis exportedas const satisfies, so this row movestypeof ERROR_CODE_LEDGER['@objectstack/plugin-security']from a 3-tuple to a 4-tuple.check:api-surfacestayed GREEN THROUGH that change, becauseapi-surface/api.jsonrecords the symbol name (ERROR_CODE_LEDGER (const)) and no signature,api-surface-signatures.jsoncovers 27define*helpers and not this const, andapi-surface-declarations/was withdrawn by #19024 and does not exist on this base. That is why this PR declaresClause-②: yesand grades@objectstack/specminor.check-widening-tellsstill fires T4 on that line withClause-②: no(exit 4), because the matcher cannot see the dedupe — it reads any new registry entry as an acceptance-set growth. The measurement above is the evidence it is false here. Reported for the owning seat rather than repaired in this PR; the declaration line is copied verbatim from the claim and is not mine to move.One corner moved with the order, declared not incidental
An ordinary duplicate attempted while no artifact source can answer now takes the lock's fail-closed
unknownrefusal —403NOT_OVERRIDABLE(PackagedPermissionSetProvenanceUnknownError, "retry once the metadata layer is readable") — instead of the 409. Both are refusals and neither writes; case 5 of the new suite pins it so a reader finds a decision rather than an accident.Tests
packages/plugins/plugin-security/src/permission-set-duplicate-name-refusal.test.ts— five cases: the ordinary duplicate's envelope (code+status+ both status spellings, never a bare "it threw" — the unfixed producer threw too), the packaged-set case answering the lock (proved by the lock's own message and bysaves.length === 0, since ADR-0005's tier gate answers the same code with a different message), the negative control that ordinary duplicates WHOSE PROVENANCE RESOLVES did not becomeNOT_OVERRIDABLE(the corner above is the case that qualifier excludes), the happy path, and the fail-closed corner.Reverse verification — two ablations, each proved on disk and restored byte-identical to
HEAD:fed1af15857f→3d92cb95990f); suite 2 failed / 3 passed — cases 2 and 5, the two ordering-dependent ones, and only those. Restored:git diff HEADempty, blob back tofed1af15857f.codestamp. Mutation landed (target 1 → 0, marker 0 → 1); suite 2 failed / 3 passed — cases 1 and 3,expected undefined to be 'UNIQUE_VIOLATION'. Restored: blob back to7b0914b1749a.Both legs ran under a
trap ... EXIT INT TERMwith absolute paths, restored throughgit checkout HEAD -- path, and verified bygit hash-objectagainst theHEADblob rather than by an exit code.Verification
Run at final head
72d68b905unless stated.pnpm --filter @objectstack/plugin-security test— 116 files / 2240 tests passed;typecheckOK (test layer 0 files / 0 errors).pnpm --filter @objectstack/spec test— 505 files / 14752 tests passed;typecheckOK;check:generated— all 15 artifacts up to date.pnpm lint— the whole repo,eslint . --no-inline-config, exit 0. Not a narrowed run, so no narrowing needs defending.scripts/pm/dispatch-gates.mjs --ran— 98 derived families, 98 run, 0 NOT-MEASURED, 0 UNRUN, each with a recorded exit code, all 0. Two of them were genuinely red mid-flight and are green only because the fix landed:check:engine-double-contract(the new double'supdatenow opens withassertEngineUpdateDispatch, and its pinned seams are registered) andcheck:objectql-double-limit(thefinddouble now holds the caller's bound by presence).Builds and test runs went through
scripts/pm/os-verify-lock.sh.Generated by Claude Code
Generated by Claude Code